Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the duplicate calendar events bug #818

Merged
merged 2 commits into from
Dec 5, 2023
Merged

Conversation

JacE070
Copy link
Contributor

@JacE070 JacE070 commented Nov 30, 2023

Summary

This change fixes the bug caused by looping over events' location.days when the calendarizeHelper has already returned events for each day.
For example, class A occurs every MWF, the calendarizeHelper returns (event for Monday) -> {..., location: {..., days: "MWF"}}, also events for Wednesday and Friday with the same location field.
Therefore, we will finally get 3*3=9 calendar events.
Finals are also added repetitively.

Test Plan

Passed the vite test. Also tested manually.

Issues

Closes #816

Copy link
Collaborator

@ap0nia ap0nia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks solid 👍

I left some suggestions for minor improvements. Once you've taken a look at everything, this should be totally fine to merge. 🚀

apps/antalmanac/src/lib/download.ts Outdated Show resolved Hide resolved
apps/antalmanac/src/lib/download.ts Outdated Show resolved Hide resolved
apps/antalmanac/src/lib/download.ts Outdated Show resolved Hide resolved
apps/antalmanac/src/lib/download.ts Outdated Show resolved Hide resolved
apps/antalmanac/src/stores/calendarizeHelpers.ts Outdated Show resolved Hide resolved
@JacE070 JacE070 requested a review from ap0nia December 4, 2023 06:35
Copy link
Member

@EricPedley EricPedley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this is a bugfix, imma just merge it real quick. @ap0nia if u want anything else to be refactored we can do it in a separate PR.

@EricPedley EricPedley merged commit 3da2adf into main Dec 5, 2023
5 of 6 checks passed
@EricPedley EricPedley deleted the export-ics-duplicate-events branch December 5, 2023 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Download ICS: Duplicate calendar events are added
3 participants